Skip to content

[dotnet] [bidi] Implement browsing context download events #16382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Oct 5, 2025
Merged

[dotnet] [bidi] Implement browsing context download events #16382

merged 4 commits into from
Oct 5, 2025

Conversation

nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Oct 5, 2025 •
edited by qodo-merge-pro bot
Loading

User description

https://w3c.github.io/webdriver-bidi/#event-browsingContext-downloadWillBegin
https://w3c.github.io/webdriver-bidi/#event-browsingContext-downloadEnd

💥 What does this PR do?

Fix downloadWillBegin event signature, implement downloadEnd event. Adding tests.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement

Description

  • Fix downloadWillBegin event signature to use proper event args

  • Implement new downloadEnd event with polymorphic event args

  • Add comprehensive tests for both download events

  • Create JSON converters for proper serialization

Diagram Walkthrough

flowchart LR
  A["BrowsingContext"] --> B["OnDownloadWillBeginAsync"]
  A --> C["OnDownloadEndAsync"]
  B --> D["DownloadWillBeginEventArgs"]
  C --> E["DownloadEndEventArgs"]
  E --> F["DownloadCanceledEventArgs"]
  E --> G["DownloadCompleteEventArgs"]
  H["JSON Converter"] --> E
  I["Tests"] --> B
  I --> C
Loading

File Walkthrough
Relevant files
Enhancement
BrowsingContext.cs
Update download event method signatures                                   

dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs

  • Fix OnDownloadWillBeginAsync method signatures to use
    DownloadWillBeginEventArgs
  • Add new OnDownloadEndAsync methods for both sync and async handlers
 +12/-2   
BrowsingContextModule.cs
Update module download event subscriptions                             

dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs

  • Fix OnDownloadWillBeginAsync method signatures to use proper event
    args
  • Add new OnDownloadEndAsync methods subscribing to
    browsingContext.downloadEnd
 +12/-2   
DownloadEndEventArgs.cs
Create download end event args classes                                     

dotnet/src/webdriver/BiDi/BrowsingContext/DownloadEndEventArgs.cs

  • Create abstract base class DownloadEndEventArgs
  • Implement DownloadCanceledEventArgs and DownloadCompleteEventArgs
    derived types
  • Include navigation info and timestamp properties
 +35/-0   
DownloadWillBeginEventArgs.cs
Create download will begin event args                                       

dotnet/src/webdriver/BiDi/BrowsingContext/DownloadWillBeginEventArgs.cs

  • Create DownloadWillBeginEventArgs record with suggested filename
  • Implement IBaseNavigationInfo interface for navigation context
 +25/-0   
DownloadEndEventArgsConverter.cs
Create polymorphic download end converter                               

dotnet/src/webdriver/BiDi/Communication/Json/Converters/Polymorphic/DownloadEndEventArgsConverter.cs

  • Implement custom JSON converter for polymorphic DownloadEndEventArgs
  • Handle canceled and complete status discriminators
  • Support deserialization to appropriate derived types
 +45/-0   
Configuration changes
Broker.cs
Register download end event converter                                       

dotnet/src/webdriver/BiDi/Communication/Broker.cs

  • Register DownloadEndEventArgsConverter for polymorphic JSON
    serialization
 +1/-0     
BiDiJsonSerializerContext.cs
Register download event types for serialization                   

dotnet/src/webdriver/BiDi/Communication/Json/BiDiJsonSerializerContext.cs

  • Add JSON serializable attributes for all download event args types
  • Include both base and derived download event classes
 +4/-0     
Tests
BrowsingContextEventsTest.cs
Add download events integration tests                                       

dotnet/test/common/BiDi/BrowsingContext/BrowsingContextEventsTest.cs

  • Add test for downloadWillBegin event with file download simulation
  • Add test for downloadEnd event verifying completion and filepath
  • Validate event args properties and types
 +65/-0   

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Oct 5, 2025
@qodo-merge-pro Qodo Merge Pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 5, 2025 •
edited
Loading

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪
Unimplemented serialization

Description: The custom JSON converter throws NotImplementedException on serialization for
DownloadEndEventArgs.Write, which could cause a runtime crash if the type is ever
serialized (e.g., logged or echoed back); consider implementing or explicitly preventing
serialization paths.
DownloadEndEventArgsConverter.cs [41-44]

Referred Code 
public override void Write(Utf8JsonWriter writer, DownloadEndEventArgs value, JsonSerializerOptions options)
{
    throw new NotImplementedException();
}
Ticket Compliance
⚪
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
⚪
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro Qodo Merge Pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 5, 2025 •
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Throw exception on unknown discriminator

In the Read method, throw a JsonException for unknown status discriminators
instead of returning null to prevent silent failures and improve error handling.

dotnet/src/webdriver/BiDi/Communication/Json/Converters/Polymorphic/DownloadEndEventArgsConverter.cs [31-39]

 public override DownloadEndEventArgs? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
 {
-    return reader.GetDiscriminator("status") switch
+    var discriminator = reader.GetDiscriminator("status");
+    return discriminator switch
     {
         "canceled" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<DownloadCanceledEventArgs>()),
         "complete" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<DownloadCompleteEventArgs>()),
-        _ => null,
+        _ => throw new JsonException($"Unknown 'status' discriminator '{discriminator}' for DownloadEndEventArgs."),
     };
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that returning null for an unknown discriminator can hide issues. Throwing an exception improves robustness by making failures explicit, which is a good practice for deserialization logic.

Medium
  • Update

@nvborisenko nvborisenko merged commit 3baf4bc into SeleniumHQ:trunk Oct 5, 2025
10 checks passed
@nvborisenko nvborisenko deleted the bidi-download-events branch October 5, 2025 13:04
This was referenced Oct 19, 2025
Merged
Closed
Merged
Merged
Closed
Closed
Closed
Merged
Merged
Closed
Open
Merged
Merged
Open
Open
Merged
Open
Merged
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

C-dotnet .NET Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nvborisenko @selenium-ci